gh-143198: fix some crashes in sqlite3.executemany with concurrent mutations
#143210
Conversation
…nt parameter iterator
picnixz
requested review from
berkerpeksag and
erlend-aasland
as code owners
Misc/NEWS.d/next/Library/2025-12-27-10-36-18.gh-issue-143198.DdIHyC.rst
Outdated
Show resolved
Hide resolved
picnixz
added
needs backport to 3.13
picnixz
changed the title
sqlite3.execute[many] with re-entrant parameter iteratorsqlite3.execute[many] with concurrent mutations
picnixz
changed the title
sqlite3.execute[many] with concurrent mutationssqlite3.executemany with concurrent mutations
|
There were more UAFs that I could find... |
picnixz
changed the title
sqlite3.executemany with concurrent mutationssqlite3.executemany with concurrent mutations
|
@serhiy-storchaka So the split went well but... it's now a larger code =/ I also changed some variable names since we're talking about parameters everywhere but it's more arguments in this case or values. I can revert the change if you want (I'll make a commit for reverting and then we can decide whether to keep the revert or not) but I wanted to show you how it would be if the implementations are separate. |
|
I feel that there are a lot of tests that are really looking-alike. It's kinda annoying to have redundancy... and it won't get any better with the other PRs for sqlite where we I need to test the callbacks. |
I understand you, the term "parameter" is used here in different meaning from what we used to. But this makes the diff unnecessary large and makes harder to follow semantic changes. Can we leave this to a separate PR? |
|
Ok, I'll revert the naming changes and all other styling stuff. And then I'll address it separately. |
| self.assertEqual(result[0][0], 3, "Basic test of Connection.executemany") | ||
| self.assertEqual(result[1][0], 4, "Basic test of Connection.executemany") | ||
|
|
||
| @subTests("params_class", (ParamsCxCloseInIterMany, ParamsCxCloseInNext)) |
There was a problem hiding this comment.
I should probably name this "params_factory"
| cx.executemany("insert into tmp(a) values (?)", params_class(cx)) | ||
|
|
||
| @subTests(("j", "n"), ([0, 1], [0, 3], [1, 3], [2, 3])) | ||
| @subTests("wtype", (list, lambda x: x)) |
There was a problem hiding this comment.
I'll use a better name here, as document what "j" and "n" are (they are just here to control when we close the connection)
3 participants
FTR, the reason why
executeis actually not affected is that the iterator we are taking is built from a list we create ourselves so it's not possible to have "bad" side effects._sqlitecursor cache via re-entrant parameter iterator #143198